feat(clipboard): add Zellij support to OSC 52 fallback - #1737

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Run the change-detection diff with sudo to avoid false-positive restarts.

At Line 20, diff may fail on permission denied for /etc/rancher/k3s/config.yaml, which makes this block copy/restart even when contents are unchanged.

-  if ! diff -q "$HOME/.config/k3s/config.yaml" /etc/rancher/k3s/config.yaml >/dev/null 2>&1; then
+  if ! $SUDO_CMD diff -q "$HOME/.config/k3s/config.yaml" /etc/rancher/k3s/config.yaml >/dev/null 2>&1; then

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@config/k3s/activate.sh` around lines 20 - 24, The diff check can fail with
permission errors when reading /etc/rancher/k3s/config.yaml causing unnecessary
copy/restart; change the check to run diff under sudo (use $SUDO_CMD) so it can
read the target file reliably (i.e., replace the current diff invocation with
$SUDO_CMD diff -q "$HOME/.config/k3s/config.yaml" /etc/rancher/k3s/config.yaml
>/dev/null 2>&1) and keep the surrounding logic (the cp and systemctl restart
calls) unchanged so restarts only occur when contents actually differ.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Use non-interactive copy flag for consistency with repo shell rules.

At Line 21, switch to cp -f to keep file operations non-interactive.

As per coding guidelines, "Always use non-interactive flags with file operations (cp -f, mv -f, rm -f, rm -rf, cp -rf) to avoid hanging on confirmation prompts".

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@config/k3s/activate.sh` at line 21, The cp invocation using $SUDO_CMD (the
line: $SUDO_CMD cp "$HOME/.config/k3s/config.yaml" /etc/rancher/k3s/config.yaml)
is missing the non-interactive flag; update this command to use cp -f to ensure
file copying is forced/non-interactive (i.e., change to use $SUDO_CMD cp -f
...), preserving the existing paths and variables.

P2: The change-detection check treats diff errors as config changes, so comparison failures can incorrectly trigger a copy and k3s restart. Run the comparison with sudo (at minimum) so permission failures don’t look like content changes.

Check if this issue is valid — if so, understand the root cause and fix it. At config/k3s/activate.sh, line 20:

<comment>The change-detection check treats `diff` errors as config changes, so comparison failures can incorrectly trigger a copy and k3s restart. Run the comparison with sudo (at minimum) so permission failures don’t look like content changes.</comment>

<file context>
@@ -17,7 +17,13 @@ fi
 if [ -n "$SUDO_CMD" ]; then
   $SUDO_CMD mkdir -p /etc/rancher/k3s
-  $SUDO_CMD cp "$HOME/.config/k3s/config.yaml" /etc/rancher/k3s/config.yaml
+  if ! diff -q "$HOME/.config/k3s/config.yaml" /etc/rancher/k3s/config.yaml >/dev/null 2>&1; then
+    $SUDO_CMD cp "$HOME/.config/k3s/config.yaml" /etc/rancher/k3s/config.yaml
+    if systemctl is-active --quiet k3s; then
</file context>

P2: The new Zellij test is not isolated: SSH_TTY/TMUX are left untouched, so it can false-pass via another OSC 52 trigger instead of validating Zellij detection.

Check if this issue is valid — if so, understand the root cause and fix it. At spec/clipboard_copy_spec.sh, line 136:

<comment>The new Zellij test is not isolated: `SSH_TTY`/`TMUX` are left untouched, so it can false-pass via another OSC 52 trigger instead of validating Zellij detection.</comment>

<file context>
@@ -120,6 +120,46 @@ The output should start with $'\033]52;c;'
+    ln -sf "$(command -v "$cmd")" "$MOCK_BIN/$cmd"
+  done
+  export PATH="$MOCK_BIN"
+  unset WAYLAND_DISPLAY
+  export ZELLIJ=0
+  export MOCK_BIN MOCK_ORIGINAL_PATH MOCK_ORIGINAL_WAYLAND MOCK_ORIGINAL_ZELLIJ
</file context>

Bare systemctl/diff may not resolve in the home-manager activation PATH. The sibling script home-manager/services/k3s/activate.sh is built via pkgs.replaceVars substituting @diff@/@systemctl@ (see home-manager/services/k3s/default.nix:11-14) precisely because activation PATH can't be relied on. This script is wired in config/k3s/default.nix as ${./activate.sh} with no substitution, so it ships bare commands.

With set -euo pipefail, both if ! diff … and if systemctl is-active … consume the exit code, so a missing binary returns 127 and the wrappers silently degrade: diff missing → cp always runs (mostly harmless), but systemctl missing → restart is skipped without any error. The net effect on a host where systemctl isn't on activation PATH is that the new tls-san: entries get copied to /etc/rancher/k3s/config.yaml but k3s is never restarted, so remote kubectl over Tailscale continues to fail TLS verification.

Fix by rendering this script through pkgs.replaceVars in config/k3s/default.nix and switching to @diff@/@systemctl@, matching the established pattern.

Global sed s|default|kyber-remote|g can corrupt cert data. k3s.yaml stores certificate-authority-data, client-certificate-data, and client-key-data as single-line base64 in the alphabet [A-Za-z0-9+/=], which contains every character of default. The substring can (rarely) appear inside the encoded blob, in which case this rewrite silently mangles the cert and kubectl fails later with a tls: failed to find any PEM data style error rather than a clean diagnostic.

Probability is low per cert, but the failure mode is hard to debug. Prefer targeted YAML-aware rename, e.g.:

sed -E 's/(name|user|cluster|current-context):[[:space:]]*default$/\1: kyber-remote/'

or pipe through yq to mutate only the cluster/user/context name fields. That keeps certificate-authority-data byte-identical no matter what the random base64 ends up containing.

TAILSCALE_IP is declared but never used — only TAILSCALE_DNS is referenced in the sed substitution below. Either drop this line or wire it in (e.g., write a second kubeconfig variant that uses https://${TAILSCALE_IP}:6443 so DNS-less clients still work).

This setup sets ZELLIJ=0 and unsets WAYLAND_DISPLAY, but doesn't unset TMUX or SSH_TTY. If the suite runs inside a tmux pane or over SSH, the OSC 52 fallback would fire from those variables and the test would pass even if the new ZELLIJ branch in clipboard-copy.sh were broken.

The "no backend" cleanup already does unset WAYLAND_DISPLAY SSH_TTY TMUX ZELLIJ (line 171) for exactly this reason — worth doing the same here:

MOCK_ORIGINAL_TMUX="${TMUX:-}"
MOCK_ORIGINAL_SSH_TTY="${SSH_TTY:-}"
…
unset WAYLAND_DISPLAY TMUX SSH_TTY
export ZELLIJ=0

and restore both in cleanup(). The pre-existing SSH_TTY block (lines 83–121) has the same gap and would benefit from the same treatment.